Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix cleanup Prow job and setup alert #3222

Merged
merged 1 commit into from
Apr 1, 2022
Merged

Fix cleanup Prow job and setup alert #3222

merged 1 commit into from
Apr 1, 2022

Conversation

chizhg
Copy link
Member

@chizhg chizhg commented Apr 1, 2022 •
edited
Loading

  1. github.com/google/go-containerregistry needs to be updated to the latest version. It's now using a very old one.
  2. gcloud auth configure-docker command is needed for running the cleanup tool
  3. Configure the cleanup Prow job to send alerts to Slack if it fails

/cc @upodroid @kvmware

@knative-prow knative-prow bot requested review from krsna-m and upodroid April 1, 2022 01:08
@knative-prow knative-prow bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Apr 1, 2022
@upodroid
Copy link
Member

upodroid commented Apr 1, 2022

This is not needed. We already authenticate to gcr.io

test-infra/images/prow-tests/Dockerfile

Line 242 in 23df246

RUN docker-credential-gcr configure-docker --registries=gcr.io,us-docker.pkg.dev

Unless we are logging in to other registries.

@knative-prow knative-prow bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Apr 1, 2022
@chizhg chizhg force-pushed the fix-cleanup branch 3 times, most recently from 14f666d to 577be5b Compare April 1, 2022 06:15
@chizhg
Copy link
Member Author

chizhg commented Apr 1, 2022 •
edited
Loading

This is not needed. We already authenticate to gcr.io

test-infra/images/prow-tests/Dockerfile

Line 242 in 23df246

RUN docker-credential-gcr configure-docker --registries=gcr.io,us-docker.pkg.dev

Unless we are logging in to other registries.

I did some more tests, and have confirmed we need two changes together to make it working:

  1. github.com/google/go-containerregistry dep version needs to be updated. It's now using a very old version.
  2. gcloud auth configure-docker needs to be run after gcloud auth activate-service-account.

I'm not entirely sure about the reason, but I have tested it at https://prow.knative.dev/view/gs/knative-prow/logs/ci-knative-cleanup-chizhg-test/1509776600835559424 and it's working.

I have updated the PR description as well.

@chizhg
Copy link
Member Author

chizhg commented Apr 1, 2022

@upodroid can I get an LGTM if this looks good to you? Thanks!

upodroid
upodroid approved these changes Apr 1, 2022
View reviewed changes
Copy link
Member

@upodroid upodroid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, large diff though

@knative-prow knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Apr 1, 2022
@knative-prow
Copy link

knative-prow bot commented Apr 1, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chizhg, upodroid

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@chizhg
Copy link
Member Author

chizhg commented Apr 1, 2022

Yeah, mostly vendor/ updates though. There are some discussions in knative/infra#134 about it.

@knative-prow knative-prow bot merged commit a0df053 into knative:main Apr 1, 2022
@knative-prow
Copy link

knative-prow bot commented Apr 1, 2022

@chizhg: Updated the job-config configmap in namespace default at cluster default using the following files:

  • key test-infra.yaml using file prow/jobs/custom/test-infra.yaml

In response to this:

  1. github.com/google/go-containerregistry needs to be updated to the latest version. It's now using a very old one.
  2. gcloud auth configure-docker command is needed for running the cleanup tool
  3. Configure the cleanup Prow job to send alerts to Slack if it fails

/cc @upodroid @kvmware

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@chizhg chizhg deleted the fix-cleanup branch April 1, 2022 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@upodroid upodroid upodroid approved these changes

@krsna-m krsna-m Awaiting requested review from krsna-m

Assignees

@upodroid upodroid

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@chizhg @upodroid